Skip to content

fix(hub-ui): clean up inactive iframe docks - #364

Merged
antfu merged 1 commit into
devframes:mainfrom
dvcolomban:dvcol/fix-iframe-pane-cleanup
Sep 8, 2026
Merged

fix(hub-ui): clean up inactive iframe docks#364
antfu merged 1 commit into
devframes:mainfrom
dvcolomban:dvcol/fix-iframe-pane-cleanup

Conversation

@dvcolomban

@dvcolomban dvcolomban commented Sep 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Background (Why)

Vue clears template refs before onUnmounted, so ViewIframe's ownership check compares the pane target with null and leaves the outgoing pane mounted. This cleanup defect was found while investigating reports of unrelated iframe content appearing when another dock fails to load.

Changes (What)

Retain the mounted target for the ownership check, preserving shared-iframe handoffs. Unmount the pane before disposing its location watcher, tolerate origin changes during subscription cleanup, and give edge mode the inactive-pane guard already used by the other layouts.

Verification (Testing)

  • 119 focused unit tests pass, including two new regressions that fail on the base.
  • Six Chromium checks pass: edge/float/standalone error handling, shared-iframe form state, navigation across synthetic origins and back, and the baseline's stale pane state.
  • Changed-file lint and hub/hub UI type checks pass with fresh package builds. Full lint reports the same ten failures on the clean base; Knip reports the same two unused dependencies.

@vercel

vercel Bot commented Sep 5, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
devframe Skipped Skipped Sep 5, 2026 12:08pm UTC

@dvcolomban
dvcolomban marked this pull request as ready for review September 5, 2026 13:47
Copilot AI lite review requested due to automatic review settings September 5, 2026 13:47
@coldtea-pr-lens

coldtea-pr-lens Bot commented Sep 5, 2026 •
edited
Loading

Copy link
Copy Markdown

◈ PR Lens

🟢 +0 new · 🟠 ~2 changed · 🔴 -0 removed · 1 flow · 4 files · commit dc953dd


Architecture

Architecture diagram for devframes/devframe at dc953dd

2 components touched across 3 lanes.

Open full size


Data flow

Data flow diagram for devframes/devframe at dc953dd

Iframe location tracking and teardown

Open full size


Drill down
Client Runtimes ＆ UI — 2 components
🟡 CHANGED Dock UI ＆ Web Component

Renders dock bars, iframe panes, and view containers with retained mount target tracking and active pane visibility management.

🟡 CHANGED Headless Client Runtime

Tracks iframe location changes across pushState and popstate events, safely detaching window listeners when navigating cross-origin.


View

  • Architecture lens
  • Data flow lens
  • Expand every detail
  • Show unchanged neighbours

Tip

Would you rather run it from CI on a key of your own? Add .github/workflows/pr-lens.yml with coldteadotai/pr-lens/packages/action@v0 and a model key in your repository secrets, say GEMINI_API_KEY. The Action asks Gemini by default, or OpenAI and any endpoint speaking /chat/completions through its provider input.

🪧 More tips
  • Run PR Lens on your own machine: npx skills add coldteadotai/pr-lens installs the agent skill. Then tell your coding agent: "Diagram the change you just made with PR Lens and attach it to the pull request."
  • Draw a diff before it is even a pull request: npx @coldtea/pr-lens-cli analyze --base origin/main reads the diff with your own model key, and npx @coldtea/pr-lens-cli render .pr-lens/graph.json draws the same lenses on your machine.
  • The boxes under View are live. Tick Architecture lens or Data flow lens to choose which diagrams appear, or Expand every detail to open every drill-down at once. The comment redraws in place a few seconds later.
  • Show unchanged neighbours lists the components this change did not touch alongside the ones it did, so the drill-down shows what the changed code sits next to.
  • GitHub will not let you zoom an image in a comment. The link under each diagram opens it full size on a page of its own, where you can.
  • The CLI's render picks up .github/pr-lens.yml automatically and applies your corrections (renames, exclusions, lane pins) at draw time.
  • PR Lens is free for open source. A star on the repository is what keeps it going.
  • Push a new commit and the whole comment re-renders for the new head. An older run never overwrites a newer one, so a slow render cannot put a stale diagram back.
  • The diagrams follow your GitHub theme, so dark mode gets the dark render and light mode the light one, and the moving dots show this pull request's data in motion.

◈ Rendered by PR Lens · crafted with ❤️ by the Coldtea team · Come say hi on Discord

Copilot AI reviewed Sep 5, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes are focused, align with the described root cause, and are backed by targeted regressions covering the previously failing scenarios.

Pull request overview

Fixes hub UI iframe-dock cleanup so shared iframes are correctly handed off during dock switches (even with Vue clearing template refs before onUnmounted), and makes iframe location tracking cleanup resilient to cross-origin navigation edge cases.

Changes:

  • Make watchFrameLocation() cleanup tolerate inaccessible prior windows/history during navigation.
  • Fix ViewIframe pane ownership checks during unmount by retaining the mounted target, and unmount before disposing the location watcher.
  • Ensure edge dock layout hides inactive iframe panes by providing the active pane key (matching other layouts), with regression tests added for cross-origin navigation scenarios.
File summaries
File Description
packages/hub/src/client/frame-location.ts Wrap listener unsubscription in try/catch to safely detach after navigation/origin changes.
packages/hub/src/client/tests/frame-location.test.ts Add regressions covering cross-origin navigation and cleanup resilience; adjust fake window behavior to simulate SecurityError.
packages/hub-ui/src/client/components/views/ViewIframe.vue Preserve mounted target for correct shared-iframe ownership checks; reorder teardown to unmount before disposing watchers.
packages/hub-ui/src/client/components/dock/DockEdge.vue Pass active-pane key into useIframePanes() so edge layout hides inactive panes like other layouts.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@antfu
antfu merged commit 78816f7 into devframes:main Sep 8, 2026
14 checks passed
@dvcolomban dvcolomban mentioned this pull request Sep 9, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dvcolomban @antfu